Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable build flag for openssl102 nix aarch64-linux #4045

Merged
merged 6 commits into from
Jun 21, 2023

Conversation

dougch
Copy link
Contributor

@dougch dougch commented Jun 7, 2023

Resolved issues:

none

Description of changes:

We're working to run integration tests on arm, but OpenSSL 1.0.2's build refuses to accept the -d debug flag.
Working around this by removing this flag for aarch64-linux only.

Call-outs:

The error:

nix develop .#openssl102                                                                                                 [18:06:19 2023-06-14]
error: builder for '/nix/store/99w05zkgabr14hywnfkj19mcmmr9pq3k-openssl-1.0.2.drv' failed with exit code 1;
       last 8 log lines:
       > unpacking sources
       > unpacking source archive /nix/store/c5dbamsp61aghc7vgw7rccw818z2l0qy-source
       > source root is source
       > patching sources
       > updateAutotoolsGnuConfigScriptsPhase
       > configuring
       > Operating system: aarch64-whatever-linux2
       > This system (debug-linux-aarch64) is not supported. See file INSTALL for details.
       For full logs, run 'nix log /nix/store/99w05zkgabr14hywnfkj19mcmmr9pq3k-openssl-1.0.2.drv'.

This appears to be a bug in the configure script of openssl102, where the $GUESSOS is expecting aarch64-whatever-linux2 vs debug-linux-aarch64 on line 674

The openssl wiki says -d isn't the only way to get a debug build, and we are using -g3:

...it's possible to quickly configure a "debug" build with ./config -d. However, you can often get into a more amicable state without the Electric Fence dependency by issuing:
./config no-asm -g3 -O0 -fno-omit-frame-pointer -fno-inline-functions"

https://wiki.openssl.org/index.php/Compilation_and_Installation#Debug_Configuration

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Jun 7, 2023
@dougch dougch force-pushed the devShell_openssl102arm branch from bf2f43f to cf30282 Compare June 7, 2023 22:00
@dougch dougch changed the title Disable building openssl102 in debug mode on nix aarch64-linux Disable building openssl102 flag on nix aarch64-linux Jun 7, 2023
@dougch dougch marked this pull request as ready for review June 7, 2023 22:34
@dougch dougch requested a review from aditishri18 June 7, 2023 22:34
@dougch dougch changed the title Disable building openssl102 flag on nix aarch64-linux Disable build flag for openssl102 nix aarch64-linux Jun 7, 2023
@dougch dougch requested a review from toidiu June 7, 2023 22:35
@dougch dougch requested a review from aditishri18 June 12, 2023 23:30
Comment on lines 18 to 23
x86_64-linux = ''
./config -d ${default_options}
'';
aarch64-linux = ''
./config ${default_options}
'';
Copy link
Contributor

@toidiu toidiu Jun 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need the -d for x86?

Can you add a comment about the -d and the bug in openssl configure script as comments since thats what most of us will be reading. You could link to this PR for more info

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make it consistent and just drop all the platform filtering, but I was trying not to affect x86 with any of these changes. Adding more words...

@dougch dougch force-pushed the devShell_openssl102arm branch from e410a04 to 6b07252 Compare June 19, 2023 19:16
@dougch dougch requested a review from toidiu June 19, 2023 19:16
@dougch dougch enabled auto-merge (squash) June 20, 2023 21:52
@dougch dougch merged commit 6592bba into aws:main Jun 21, 2023
@dougch dougch mentioned this pull request Jun 21, 2023
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants